-
-
Notifications
You must be signed in to change notification settings - Fork 425
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(datepicker): datePicker now supports formatting of date #1314
base: main
Are you sure you want to change the base?
Conversation
Run & review this pull request in StackBlitz Codeflow. |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe recent changes enhance the Datepicker component by adding localization and customizable date formats, leveraging Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files ignored due to path filters (1)
packages/ui/package.json
is excluded by:!**/*.json
Files selected for processing (7)
- apps/web/content/docs/components/datepicker.mdx (1 hunks)
- apps/web/examples/datepicker/datepicker.format.tsx (1 hunks)
- apps/web/examples/datepicker/index.ts (1 hunks)
- packages/ui/src/components/Datepicker/Datepicker.spec.tsx (5 hunks)
- packages/ui/src/components/Datepicker/Datepicker.stories.tsx (1 hunks)
- packages/ui/src/components/Datepicker/Datepicker.tsx (4 hunks)
- packages/ui/src/components/Datepicker/helpers.ts (3 hunks)
Additional comments: 5
apps/web/examples/datepicker/index.ts (1)
- 2-2: The addition of the
format
export fromdatepicker.format
is correctly implemented.apps/web/examples/datepicker/datepicker.format.tsx (1)
- 1-42: The new example file
datepicker.format.tsx
correctly demonstrates the usage of theinputFormat
prop with theDatepicker
component. The structure and export of the example are consistent with the project's standards.packages/ui/src/components/Datepicker/Datepicker.stories.tsx (1)
- 64-77: The addition of the
FormattedDate
story inDatepicker.stories.tsx
correctly showcases the newinputFormat
prop with a sample format. This enhances the documentation and provides a practical example of the new feature.apps/web/content/docs/components/datepicker.mdx (1)
- 22-29: The addition of the documentation section for setting the date format using the
inputFormat
prop is clear and informative. The note on format conventions is particularly helpful for guiding users in correctly specifying date formats.packages/ui/src/components/Datepicker/Datepicker.tsx (1)
- 100-100: The addition of the
inputFormat
property to theDatepickerProps
interface and its usage inDatepickerRender
are correctly implemented. The default value and its application in date formatting calls enhance the component's flexibility and align with the PR objectives.Also applies to: 121-121, 206-206, 209-209, 277-277
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files ignored due to path filters (1)
packages/ui/package.json
is excluded by:!**/*.json
Files selected for processing (5)
- apps/web/examples/datepicker/datepicker.format.tsx (1 hunks)
- packages/ui/src/components/Datepicker/Datepicker.spec.tsx (5 hunks)
- packages/ui/src/components/Datepicker/Datepicker.tsx (3 hunks)
- packages/ui/src/components/Datepicker/helpers.spec.tsx (1 hunks)
- packages/ui/src/components/Datepicker/helpers.ts (3 hunks)
Files skipped from review as they are similar to previous changes (4)
- apps/web/examples/datepicker/datepicker.format.tsx
- packages/ui/src/components/Datepicker/Datepicker.spec.tsx
- packages/ui/src/components/Datepicker/Datepicker.tsx
- packages/ui/src/components/Datepicker/helpers.spec.tsx
Additional comments: 1
packages/ui/src/components/Datepicker/helpers.ts (1)
- 1-1: The import of
DateFNSFormat
from 'date-fns' is correctly added to support date formatting functionality.
btw, what code editor are u using @dhavalveera ? It seems to me that u are not formatting the code on-save, or at all. |
I'm using VSCode, and when I do save ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (9)
- apps/web/content/docs/components/datepicker.mdx (1 hunks)
- apps/web/examples/datepicker/datepicker.format.tsx (1 hunks)
- apps/web/examples/datepicker/datepicker.localization.tsx (1 hunks)
- apps/web/examples/datepicker/index.ts (1 hunks)
- packages/ui/src/components/Datepicker/Datepicker.spec.tsx (5 hunks)
- packages/ui/src/components/Datepicker/Datepicker.stories.tsx (1 hunks)
- packages/ui/src/components/Datepicker/Datepicker.tsx (4 hunks)
- packages/ui/src/components/Datepicker/helpers.spec.tsx (1 hunks)
- packages/ui/src/components/Datepicker/helpers.ts (3 hunks)
Files skipped from review as they are similar to previous changes (5)
- apps/web/examples/datepicker/datepicker.format.tsx
- apps/web/examples/datepicker/index.ts
- packages/ui/src/components/Datepicker/Datepicker.spec.tsx
- packages/ui/src/components/Datepicker/Datepicker.stories.tsx
- packages/ui/src/components/Datepicker/Datepicker.tsx
Additional comments: 11
apps/web/examples/datepicker/datepicker.localization.tsx (3)
- 10-10: The update to the
language
attribute and label attributes for the Datepicker component enhances consistency and ensures compatibility with thedate-fns
package.- 18-18: The consistency in updating the
language
attribute and label attributes across different parts of the file ensures uniformity in the component's usage.- 23-23: The consistency in updating the
language
attribute and label attributes across different parts of the file ensures uniformity in the component's usage.apps/web/content/docs/components/datepicker.mdx (2)
- 22-30: The addition of a section for setting the date format using the
inputFormat
prop, along with notes on date format conventions and the reliance ondate-fns
, is crucial for informing users about the new formatting capabilities of the Datepicker component.- 38-38: The note on the reliance of the
Datepicker
component ondate-fns
for localization, and guiding users to refer to available localizations fromdate-fns
, is important for correctly utilizing the localization feature.packages/ui/src/components/Datepicker/helpers.ts (3)
- 1-2: The addition of imports for
DateFNSFormat
andDateFNSLocale
is necessary for enabling the new date formatting feature, aligning with the objective of providing customizable date formatting capabilities.- 4-5: Defining a type for available locales enhances type safety and clarity in the code, supporting the new date formatting feature by specifying the types of locales that can be used.
- 105-116: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [108-127]
The modification of the
getFormattedDate
function to include adateFormat
parameter and utilizeDateFNSFormat
for date formatting aligns with best practices for localization support. The approach to determining the locale ensures that the correct locale object is used for formatting.packages/ui/src/components/Datepicker/helpers.spec.tsx (3)
- 157-157: The update to include an additional argument for the date format string in the
getFormattedDate
function call is necessary for testing the new formatting capabilities. The specified format matches the expected output, ensuring the test correctly assesses the functionality.- 164-164: The update to include an additional argument for the date format string in the
getFormattedDate
function call, along with specified options, is necessary for testing the new formatting capabilities. The approach is consistent with the objective of testing the new feature.- 170-170: The update to test the localization feature by specifying a different language code is important for ensuring the feature works as expected across different locales. This is crucial for testing the localization aspect of the new feature.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (1)
- packages/ui/package.json (1 hunks)
Additional Context Used
Additional comments not posted (1)
packages/ui/package.json (1)
80-80
: The addition ofdate-fns
version3.6.0
to the dependencies is correctly implemented and aligns with the PR's objectives. Using a fixed version ensures stability but consider the trade-offs between fixed versions and version ranges for future maintenance.
@dhavalveera could you please rebase it with main? I want to get this one approved. :) |
@rluders --- this conflicts, can't be done from my end, as I don't have the write access to this repository. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- packages/ui/package.json (1 hunks)
Additional comments not posted (2)
packages/ui/package.json (2)
64-64
: The addition ofdate-fns
with a fixed version number3.6.0
aligns with the PR's objectives to enhance date formatting capabilities in the DatePicker component.
64-64
: The addition ofclean-package
is a good practice for keeping the package lightweight for production. Ensure that thepostpublish
script correctly restores removed sections.Verification successful
The
postpublish
script is correctly configured to runclean-package restore
, which should restore the removed sections. However, we should verify theclean-package
script's configuration to ensure it is set up properly.
The
clean-package
script is correctly configured in thescripts
section and is set to run duringprepack
. Thepostpublish
script is also correctly set to restore the removed sections.
- packages/ui/package.json
"prepack": "clean-package"
"postpublish": "clean-package restore"
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correct configuration of `clean-package` in the `postpublish` script. # Test: Check the postpublish script for restoring the removed sections. rg --type json 'postpublish' packages/ui/package.jsonLength of output: 100
Script:
#!/bin/bash # Description: Verify the configuration of the `clean-package` script in the `scripts` section. # Test: Check the scripts section for the `clean-package` script configuration. rg --type json 'clean-package' packages/ui/package.jsonLength of output: 177
Hello @SutuSebastian , currently I'm facing an issue is like e.g. for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Outside diff range and nitpick comments (1)
packages/ui/src/components/Datepicker/helpers.ts (1)
Line range hint
108-128
: **** Incorrect default parameter position and potential locale handling issue.
- Parameter Order: The default parameter
language
should not precede required parameters. This can lead to confusion and errors in function calls.- Locale Handling: The way locales are handled might lead to unexpected behaviors if the locale code does not match exactly with
DateFNSLocale
entries.- export const getFormattedDate = ( - language: AvailableLocales = "enUS", - date: Date, - options?: Intl.DateTimeFormatOptions, - dateFormat: string = "dd-MMM-yyyy", - ): string => { + export const getFormattedDate = ( + date: Date, + dateFormat: string = "dd-MMM-yyyy", + language: AvailableLocales = "enUS", + options?: Intl.DateTimeFormatOptions + ): string => { - const getLocale = - language === "enUS" ? DateFNSLocale["enUS"] : Object.values(DateFNSLocale).find((l) => l.code === language); + const getLocale = DateFNSLocale[language] || DateFNSLocale.enUS;
- Parameter Order: Rearrange parameters to place the default parameter last.
- Locale Handling: Simplify the locale retrieval logic to directly access locales from
DateFNSLocale
and provide a fallback toenUS
if the specified locale is not found.Tools
Biome
[error] 109-109: This default parameter should follow the last required parameter or should be a required parameter. (lint/style/useDefaultParameterLast)
The last required parameter is here:
A default parameter that precedes a required parameter cannot be omitted at call site.
Unsafe fix: Turn the parameter into a required parameter.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
bun.lockb
is excluded by!**/bun.lockb
Files selected for processing (2)
- package.json (1 hunks)
- packages/ui/src/components/Datepicker/helpers.ts (3 hunks)
Files skipped from review due to trivial changes (1)
- package.json
Additional context used
Learnings (1)
packages/ui/src/components/Datepicker/helpers.ts (1)
User: rluders PR: themesberg/flowbite-react#1314 File: packages/ui/src/components/Datepicker/helpers.ts:112-119 Timestamp: 2024-03-25T13:03:57.322Z Learning: The `getFormattedDate` function in `packages/ui/src/components/Datepicker/helpers.ts` should use `DateFNSFormat` directly with the date and format string for proper localization support, mapping language codes to locale objects from `date-fns/locale`.
Biome
packages/ui/src/components/Datepicker/helpers.ts
[error] 109-109: This default parameter should follow the last required parameter or should be a required parameter. (lint/style/useDefaultParameterLast)
The last required parameter is here:
A default parameter that precedes a required parameter cannot be omitted at call site.
Unsafe fix: Turn the parameter into a required parameter.
Additional comments not posted (1)
packages/ui/src/components/Datepicker/helpers.ts (1)
5-5
: **** Good use of TypeScript for defining type aliases.Defining
AvailableLocales
as a type alias for locale keys indate-fns/locale
ensures type safety and better integration with thedate-fns
library.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- packages/ui/package.json (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- packages/ui/package.json
Any news on this ? |
any update ? |
needs a re-work after #1190 is merged |
🛠️ Enhancement: Added Date Formatting Support to DatePicker
This PR introduces date formatting support to the DatePicker component by leveraging the
date-fns
package. Users can now customize date formats such asdd-MMM-yyyy
ordd MMM yyyy
according to their preferences.outputs/examples:
dd-MMM-yyyy // output => 11-Mar-2024
dd MMM yyyy // output => 11 Mar 2024
dd-MM-yyyyy // output => 11-03-2024
Summary by CodeRabbit
New Features
Improvements
date-fns
.Dependencies
date-fns
version3.6.0
to support date formatting and localization.